[Merged by Bors] - Implement slashing protection interchange format #1544

michaelsproul · 2020-08-18T09:09:41Z

Issue Addressed

Implements support for importing and exporting the slashing protection DB interchange format described here:

https://hackmd.io/@sproul/Bk0Y0qdGD

Also closes #1584

Proposed Changes

Support for serializing and deserializing the format
Support for importing and exporting Lighthouse's database
CLI commands to invoke import and export
Export to minimal format (required when a minimal format has been previously imported)
Tests for export to minimal (utilising mixed importing and attestation signing?)
Tests for import/export of complete format, and import of minimal format
~~Prevent attestations with sources less than our max source (Danny's suggestion). Required for the fake attestation that we put in for the minimal format to block attestations from source 0.~~
Add the concept of a "low watermark" for compatibility with the minimal format

Bonus!

A fix to a potentially nasty bug involving validators getting re-registered each time the validator client ran! Thankfully, the ordering of keys meant that the validator IDs used for attestations and blocks remained stable -- otherwise we could have had some slashings on our hands! 😱
Tests to confirm that this bug is indeed vanquished

djrtwo · 2020-08-20T13:09:26Z

Are you implementing both formats from the doc? Or should teku instead just use the first format and just provide one stubbed message for attestations and blocks

michaelsproul · 2020-08-20T23:21:28Z

@djrtwo Yeah I've already implemented both! I just need to write some tests and a CLI porcelain, but have been distracted by fixing some DB issues

michaelsproul · 2020-08-27T08:02:15Z

I'm planning to try a different approach to importing the minimal format. Rather than filling the DB with fake blocks for every slot from 0 to last_signed_block_slot (which is really slow), I want to add the last signed block slot as a first-class entity in slashing protection, which we can set with one query. This would also allow us to prune the DB, although I might not implement that yet.

A similar low watermark for attestations will obsolete the need for the source >= max_source check on attestations, and the "clever" attestation import strategy with an attestation spanning 0->last_signed_attestation_target_epoch. I realised that this strategy was going to get even messier, because in addition to the attestation from 0->N which blocks i->j for i, j in [1..N], we need one with a source greater than 0 to block 0->j for j in 1..N that isn't mutually slashable with the 0->N one. I think there's a hacky way to do it with 2 attestations, but the low watermark really seems simpler and less error prone.

Glossary:

i->j is an attestation with source epoch i and target j

michaelsproul · 2020-09-06T23:36:02Z

Blocked on #1588

## Proposed Changes This is an extraction of the quoted int code from #1569, that I've come to rely on for #1544. It allows us to parse integers from serde strings in YAML, JSON, etc. The main differences from the code in Paul's original PR are: * Added a submodule that makes quoting mandatory (`require_quotes`). * Decoding is generic over the type `T` being decoded. You can use `#[serde(with = "serde_utils::quoted_u64::require_quotes")]` on `Epoch` and `Slot` fields (this is what I do in my slashing protection PR). I've turned on quoting for `Epoch` and `Slot` in this PR, but will leave the other `types` changes to you Paul. I opted to put everything in the `conseus/serde_utils` module so that BLS can use it without a circular dependency. In future when we want to publish `types` I think we could publish `serde_utils` as `lighthouse_serde_utils` or something. Open to other ideas on this front too.

michaelsproul · 2020-09-07T07:42:28Z

This is ready for review!

There's one FIXME that depends on #1532, but that should be straight-forward to resolve once one of these PRs is merged.

michaelsproul · 2020-09-17T23:35:07Z

Marking this as WIP again, because it needs updating for v4 of the spec

michaelsproul · 2020-09-28T06:21:50Z

This is ready to roll again, just waiting on resolution of merge conflicts in #1532

michaelsproul · 2020-09-28T06:22:55Z

This PR closes an A0 issue (#1584), but the fix is relatively well-contained in 1dbd537 if we want to cherry-pick it and merge it before reviewing+merging this PR.

Also fix a bug with double registration of validators

paulhauner

Impressive, especially with all the test vectors and interop contribution!

I just have a few comments. I played around with it and it worked great, I had a couple of suggestions for logs that might reduce our support load.

account_manager/src/validator/slashing_protection.rs

validator_client/slashing_protection/src/slashing_database.rs

account_manager/src/validator/slashing_protection.rs

lighthouse/tests/account_manager.rs

validator_client/slashing_protection/build.rs

validator_client/slashing_protection/src/interchange.rs

michaelsproul · 2020-10-02T01:09:52Z

All comments addressed 🎉

paulhauner

Approved! 🎉

michaelsproul · 2020-10-02T01:42:09Z

Thanks!

bors r+

## Issue Addressed Implements support for importing and exporting the slashing protection DB interchange format described here: https://hackmd.io/@sproul/Bk0Y0qdGD Also closes #1584 ## Proposed Changes * [x] Support for serializing and deserializing the format * [x] Support for importing and exporting Lighthouse's database * [x] CLI commands to invoke import and export * [x] Export to minimal format (required when a minimal format has been previously imported) * [x] Tests for export to minimal (utilising mixed importing and attestation signing?) * [x] Tests for import/export of complete format, and import of minimal format * [x] ~~Prevent attestations with sources less than our max source (Danny's suggestion). Required for the fake attestation that we put in for the minimal format to block attestations from source 0.~~ * [x] Add the concept of a "low watermark" for compatibility with the minimal format Bonus! * [x] A fix to a potentially nasty bug involving validators getting re-registered each time the validator client ran! Thankfully, the ordering of keys meant that the validator IDs used for attestations and blocks remained stable -- otherwise we could have had some slashings on our hands! 😱 * [x] Tests to confirm that this bug is indeed vanquished

bors · 2020-10-02T02:47:57Z

Pull request successfully merged into v0.3.0-staging.

Build succeeded:

michaelsproul added work-in-progress PR is a work-in-progress val-client Relates to the validator client binary security labels Aug 18, 2020

wemeetagain mentioned this pull request Aug 28, 2020

Implement Slashing Protection Import/Export ChainSafe/lodestar#1471

Closed

michaelsproul force-pushed the slashing-db-interchange branch from 428304a to 6d77a3a Compare August 31, 2020 02:02

michaelsproul mentioned this pull request Sep 3, 2020

[Merged by Bors] - Implement standard eth2.0 API #1569

Closed

15 tasks

michaelsproul force-pushed the slashing-db-interchange branch 2 times, most recently from f50c3a4 to 0be58dc Compare September 3, 2020 07:16

michaelsproul mentioned this pull request Sep 5, 2020

[Merged by Bors] - Add serde_utils module with quoted u64 support #1588

Closed

michaelsproul force-pushed the slashing-db-interchange branch from 0be58dc to a49abeb Compare September 6, 2020 23:35

michaelsproul marked this pull request as ready for review September 6, 2020 23:35

michaelsproul added the blocked label Sep 6, 2020

michaelsproul force-pushed the slashing-db-interchange branch from a49abeb to 1701c1a Compare September 7, 2020 02:15

michaelsproul added ready-for-review The code is ready for review and removed blocked work-in-progress PR is a work-in-progress labels Sep 7, 2020

michaelsproul requested a review from paulhauner September 7, 2020 07:22

michaelsproul mentioned this pull request Sep 11, 2020

Register validators with slashing protection as they are created/imported #1584

Closed

michaelsproul added work-in-progress PR is a work-in-progress and removed ready-for-review The code is ready for review labels Sep 17, 2020

paulhauner added the A1 label Sep 23, 2020

michaelsproul changed the base branch from master to v0.3.0-staging September 28, 2020 06:20

michaelsproul removed the work-in-progress PR is a work-in-progress label Sep 28, 2020

michaelsproul added A0 blocked and removed A1 labels Sep 28, 2020

paulhauner force-pushed the v0.3.0-staging branch from 0a1f1b8 to 0924a60 Compare September 28, 2020 07:25

michaelsproul force-pushed the slashing-db-interchange branch from 1dbd537 to f80fcb3 Compare September 29, 2020 02:15

michaelsproul added ready-for-review The code is ready for review and removed blocked labels Sep 29, 2020

paulhauner force-pushed the v0.3.0-staging branch from 55107d0 to 29709c5 Compare September 29, 2020 10:22

michaelsproul force-pushed the slashing-db-interchange branch from 60765fb to 86730f3 Compare September 30, 2020 04:11

paulhauner force-pushed the v0.3.0-staging branch from 1ad28c0 to 22aedda Compare October 1, 2020 01:14

michaelsproul added 4 commits October 1, 2020 11:23

Register validators on create/import

fa706e8

Also fix a bug with double registration of validators

Implement slashing protection interchange format

029059e

Update Cargo.lock

f345d5d

Fix Clippy lints

07f93ba

michaelsproul force-pushed the slashing-db-interchange branch from ce55656 to 07f93ba Compare October 1, 2020 01:24

michaelsproul mentioned this pull request Oct 1, 2020

[Merged by Bors] - Implement VC API #1657

Closed

18 tasks

Fix .gitignore bin

e82f9e4

paulhauner requested changes Oct 1, 2020

View reviewed changes

michaelsproul added 2 commits October 1, 2020 16:53

Make imports atomic, address review comments

953788b

Check slashing protection in account CLI tests

3c62cf5

paulhauner approved these changes Oct 2, 2020

View reviewed changes

bors bot changed the title ~~Implement slashing protection interchange format~~ [Merged by Bors] - Implement slashing protection interchange format Oct 2, 2020

bors bot closed this Oct 2, 2020

michaelsproul deleted the slashing-db-interchange branch October 2, 2020 02:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Implement slashing protection interchange format #1544

[Merged by Bors] - Implement slashing protection interchange format #1544

michaelsproul commented Aug 18, 2020 •

edited

Loading

djrtwo commented Aug 20, 2020

michaelsproul commented Aug 20, 2020

michaelsproul commented Aug 27, 2020

michaelsproul commented Sep 6, 2020

michaelsproul commented Sep 7, 2020

michaelsproul commented Sep 17, 2020

michaelsproul commented Sep 28, 2020

michaelsproul commented Sep 28, 2020

paulhauner left a comment

michaelsproul commented Oct 2, 2020

paulhauner left a comment

michaelsproul commented Oct 2, 2020

bors bot commented Oct 2, 2020

[Merged by Bors] - Implement slashing protection interchange format #1544

[Merged by Bors] - Implement slashing protection interchange format #1544

Conversation

michaelsproul commented Aug 18, 2020 • edited Loading

Issue Addressed

Proposed Changes

djrtwo commented Aug 20, 2020

michaelsproul commented Aug 20, 2020

michaelsproul commented Aug 27, 2020

michaelsproul commented Sep 6, 2020

michaelsproul commented Sep 7, 2020

michaelsproul commented Sep 17, 2020

michaelsproul commented Sep 28, 2020

michaelsproul commented Sep 28, 2020

paulhauner left a comment

Choose a reason for hiding this comment

michaelsproul commented Oct 2, 2020

paulhauner left a comment

Choose a reason for hiding this comment

michaelsproul commented Oct 2, 2020

bors bot commented Oct 2, 2020

michaelsproul commented Aug 18, 2020 •

edited

Loading